Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Update broccoli-typescript-compiler to v4.1.0 #894

Merged
merged 8 commits into from
Dec 24, 2018

Conversation

Turbo87
Copy link
Member

@Turbo87 Turbo87 commented Dec 24, 2018

This PR attempts to update the broccoli-typescript-compiler dependency to v4.1.0, which internally updates TypeScript from v2.8.3 to v3.2.2. This results in a few different types of errors, most of which got resolved by #889 already. There are two types of errors remaining:

  • use of foreignobject instead of foreignObject as a tag name string (fixed in this PR)
  • broken type declarations in @glimmer/syntax/lib/types/nodes.ts (I've spent 2-3 hours trying to get this to work but still fail to understand how this is supposed to work)

I've given up trying to fix the nodes.ts issue, which is why I opened this PR as WIP.

@krisselden according to git you wrote the type declarations in that file. would be great if you could have a look.

This PR will be needed to unblock emberjs/ember.js#17298

UPDATE: After another 2 hours, I seem to have found a solution to the nodes.ts issues. Afterward, I also discovered other issues which are fixed by #896, #895 and two more commits in this PR.

/cc @tomdale @rwjblue

@Turbo87 Turbo87 requested a review from krisselden December 24, 2018 10:23
@Turbo87 Turbo87 force-pushed the tsc-next branch 2 times, most recently from 1b3948c to 15f3633 Compare December 24, 2018 12:21
@Turbo87 Turbo87 changed the title WIP: Update broccoli-typescript-compiler to v4.1.0 Update broccoli-typescript-compiler to v4.1.0 Dec 24, 2018
@Turbo87 Turbo87 removed the request for review from krisselden December 24, 2018 13:37
@rwjblue
Copy link
Member

rwjblue commented Dec 24, 2018

Needs a rebase and is also blocked on addressing my comment in #895...

@rwjblue
Copy link
Member

rwjblue commented Dec 24, 2018

Ok, I think we only need one last rebase?

@Turbo87
Copy link
Member Author

Turbo87 commented Dec 24, 2018

@rwjblue done already 😁

@rwjblue
Copy link
Member

rwjblue commented Dec 24, 2018

Added breaking label (since updating TS should be considered a breaking change).

@rwjblue rwjblue merged commit e74eabc into glimmerjs:master Dec 24, 2018
@Turbo87 Turbo87 deleted the tsc-next branch December 24, 2018 14:45
@Thom4sGoh

This comment has been minimized.

wycats added a commit that referenced this pull request Jan 8, 2019
wycats added a commit that referenced this pull request Jan 9, 2019
wycats added a commit that referenced this pull request Jan 9, 2019
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants